Skip to content

Hydrate from attributes + ids on create() instead of post-insert read#25

Merged
alexstandiford merged 1 commit into
mainfrom
feat/hydrate-from-attributes
Apr 30, 2026
Merged

Hydrate from attributes + ids on create() instead of post-insert read#25
alexstandiford merged 1 commit into
mainfrom
feat/hydrate-from-attributes

Conversation

@alexstandiford
Copy link
Copy Markdown
Contributor

Problem

WithDatastoreHandlerMethods::create() does an INSERT then immediately reads the row back to hydrate the returned model. On any host with a write/read-split router in front of MySQL/MariaDB (ProxySQL, MaxScale, RDS Proxy, Aurora, Cloudways Autonomous, etc.), the autocommit INSERT lands on master and the autocommit SELECT is allowed to be routed to a read-replica that hasn't yet received the write. The result is a RecordNotFoundException thrown from a successful insert.

This is a property of distributed systems, not of MariaDB specifically. Any future PHPNomad integration against an eventually-consistent backend (Postgres + pgpool, MongoDB with read preferences, Cassandra, Elasticsearch, REST behind a CDN) would exhibit the same failure shape.

Production reproduction

Reproduced on a customer's actual production environment (Cloudways Autonomous, MariaDB 10.11.5 behind a routed K8s service):

Mode Failures Notes
baseline (autocommit) 7 / 500 Today's framework code path
txn_per_op (autocommit) 7 / 500
big_txn (START TRANSACTIONCOMMIT) 0 / 500
read_uncommitted (in txn) 0 / 500
serializable (in txn) 0 / 200

Every failure had the same client-side CONNECTION_ID(), with LAST_INSERT_ID() returning the correct id, recovering within 10–50ms. The router pins clients to one backend during a transaction (mandatory for txn correctness), which is why every transactional mode passed cleanly.

Approach

Most ORMs and platforms (WordPress core, Eloquent, Doctrine, ActiveRecord) avoid this race by not reading back after a write — they construct the model from the data the caller passed plus the new identity, and trust their in-memory representation. This PR brings PHPNomad in line with that pattern.

What changed

Column gains an optional withPhpDefault() setter

$column = (new Column('createdAt', 'TIMESTAMP', null, 'NOT NULL DEFAULT CURRENT_TIMESTAMP'))
    ->withPhpDefault(static fn () => (new DateTimeImmutable())->format('Y-m-d H:i:s'));

The callable is invoked at create() time for any column the caller didn't provide. The DB-side DEFAULT stays in place as belt-and-suspenders for inserts that bypass the framework.

DateCreatedFactory and DateModifiedFactory use it

Both factories now provide a phpDefault that returns the current MySQL-format timestamp. Existing column DDL is unchanged.

create() hydrates from attributes + ids

$attributes = $this->applyPhpDefaults($attributes);
$ids = $this->serviceProvider->queryStrategy->insert($this->table, $attributes);
$result = $this->modelAdapter->toModel(Arr::merge($attributes, $ids));
$this->cacheItems([$result]);
$this->serviceProvider->eventStrategy->broadcast(new RecordCreated($result));

No post-insert SELECT. The cache pre-warm lands the model under the same key the existing getModels() flow would have used, so subsequent reads transparently short-circuit through the cache.

Contract change

Any column whose value should appear in the post-create() model must either:

  • be passed by the caller, or
  • come from a column factory that provides a phpDefault.

Columns relying solely on a DB-side DEFAULT (without a matching phpDefault), trigger-set values, and generated columns won't be reflected in the in-memory model. Callers that need those values should refresh explicitly. In practice this is a non-event for existing PHPNomad consumers because all framework-provided default columns flow through the factories updated here.

Why this is better than the writer-consistent approach

The previous attempt (#21's predecessor, plus the closed PRs in phpnomad/wordpress-integration#30 and phpnomad/mysql-db-integration#12) wrapped post-write reads in a transaction or used a HyperDB hook to force master routing. That works but:

  • adds latency to every create() even on hosts with no router
  • has a serious nested-transaction footgun (silently commits a caller-owned outer transaction on MariaDB)
  • only solves the problem for SQL backends with router awareness — useless against MongoDB/Cassandra/Elasticsearch/REST
  • adds connection-pinning pressure on routers under load

Hydrating from attributes:

  • zero overhead on every host (no transaction, no extra round-trip)
  • architecturally eliminates the race rather than mitigating it
  • works the same way against any future integration regardless of backend
  • matches the pattern WordPress/Eloquent/Doctrine already use

Tests

  • Column::withPhpDefault / getPhpDefault contract
  • DateCreatedFactory and DateModifiedFactory produce a working phpDefault
  • create() hydrates without calling query() (no readback)
  • create() applies phpDefaults for missing columns
  • Caller-provided values win over phpDefaults
PHPUnit 9.6.22
.............                                                     13 / 13 (100%)
OK (13 tests, 34 assertions)

PHPStan level 9 clean for the changed files. Pre-existing warnings on Column.php (about iterable type specs on the original variadic $attributes) are untouched.

Related

  • Closes phpnomad/wordpress-integration#30 and phpnomad/mysql-db-integration#12 — those defensive PRs can stay closed; this addresses the underlying problem at the right layer.

The post-insert read in WithDatastoreHandlerMethods::create() races read-replicas
on hosts with write/read-split routers (ProxySQL, MaxScale, RDS Proxy, Aurora,
Cloudways Autonomous, etc.). The router routes the autocommit INSERT to master
and the immediately-following autocommit SELECT to a replica that hasn't yet
received the write, producing RecordNotFoundException after a successful
insert. Reproduced live on a customer environment: 14/2200 sequential creates
failed (~0.6%), all on the same client connection id, all with LAST_INSERT_ID()
returning the right id, all recovering within 10-50ms. Wrapping in an explicit
transaction eliminated the failure entirely (because routers pin transactional
clients to one backend), but adding transactions to every create has its own
trade-offs - and the underlying problem shows up on any backend with eventual
consistency (Postgres+pgpool, MongoDB read preferences, Cassandra, search
indexes, REST APIs behind CDNs).

The cleanest fix is to remove the post-insert read entirely. We already have
the data the caller passed in, plus the new identity from the insert response.
Construct the model from those in PHP and skip the round-trip.

What changed:

- Column gains an optional withPhpDefault(callable) setter. The callable is
  invoked at create time for any column the caller didn't provide, so the
  in-memory model carries the same value the DB would have defaulted in.
  Backward compatible: existing columns without a phpDefault behave identically.

- DateCreatedFactory and DateModifiedFactory now provide phpDefaults that
  return the current MySQL-format timestamp. The DB-side DEFAULT CURRENT_TIMESTAMP
  stays in place as belt-and-suspenders coverage for inserts that bypass the
  framework.

- WithDatastoreHandlerMethods::create() applies phpDefaults to the attributes
  array, runs the insert, and hydrates the model via modelAdapter->toModel(
  array_merge(attributes, ids)). No second SELECT. Cache pre-warm via the
  existing cacheItems() so subsequent reads short-circuit through the cache
  the same way they do today.

The new contract: any column whose value should appear in the post-create()
model must either be passed by the caller or come from a column factory that
provides a phpDefault. Trigger-set values, generated columns, and DB defaults
without a matching phpDefault won't be reflected in the in-memory model -
they'll need an explicit refresh by the caller if they're load-bearing.

Tests cover:
- Column.withPhpDefault / getPhpDefault contract
- DateCreatedFactory and DateModifiedFactory produce a working phpDefault
- create() hydrates without calling query() (no readback)
- create() applies phpDefaults for missing columns
- Caller-provided values win over phpDefaults

Closes the underlying race that motivated phpnomad/wordpress-integration#30
and phpnomad/mysql-db-integration#12 - both can stay closed.
@alexstandiford alexstandiford merged commit 7037ef4 into main Apr 30, 2026
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant